-
Notifications
You must be signed in to change notification settings - Fork 3.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
op-plasma: track challenges when pipeline is stalled #9856
op-plasma: track challenges when pipeline is stalled #9856
Conversation
WalkthroughWalkthroughThe changes encompass enhancements in timing and block number parameters within a plasma test suite, improvements in challenge tracking and expiration logic, and optimizations in managing plasma data sources. Notably, there is a focus on handling sequencer stalls and finalization processes within plasma-based data availability (DA) windows. Logging and comments have been refined for clarity, and new test functions have been introduced to ensure robust challenge expiration and filter invalid block numbers effectively. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests look fine, but the logic in the da mgr / da state is kinda complex & I think subtly broken.
I think to make it clearer, we should maintain two queues (that also have a map to check for set membership). One should be for the commitments we see on L1, the other should be for the challenges we see on L1. When we expire a commitment, we can check the challenges to see if one is around & vice-versa.
Or maybe we even need to go up to three queues: unchallenged comms, challenges, & challenged comms. Right now the canonical boolean is tricky & the multiple entrypoints to storing data are hard to reason about.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like it's properly tracking the challenges to me.
Co-authored-by: Joshua Gutow <[email protected]>
Head branch was pushed to by a user without write access
f3d0102
…sm#9856) * fix: track challenges when pipeline is stalled * cleanup * fix: remove tx version byte in test * remove Heap usage * fix: add pending queue * remove unused check * use 2 PQs * Update comment in op-e2e/actions/plasma_test.go Co-authored-by: Joshua Gutow <[email protected]> --------- Co-authored-by: Joshua Gutow <[email protected]>
Description
This PR adds tracking of challenge state beyond the derivation pipeline origin. In the case where derivation is stalled due to missing data pending a challenge, we keep indexing challenge events related to future commitments. These challenges however will be ignored and evicted if they are never reattached to valid commitments in the batcher inbox.
Tests